Skip to content

fix: normalize view definitions for PostgreSQL 14-15 compatibility#138

Merged
tianzhou merged 1 commit intomainfrom
fix_view
Oct 31, 2025
Merged

fix: normalize view definitions for PostgreSQL 14-15 compatibility#138
tianzhou merged 1 commit intomainfrom
fix_view

Conversation

@tianzhou
Copy link
Contributor

PostgreSQL 14 and 15's pg_get_viewdef() returns table-qualified column names when views are created with schema-qualified table references in the FROM clause. PostgreSQL 16+ simplified this behavior to return unqualified column names.

Root Cause:

  • When a view is created with FROM public.dept_emp, pg_get_viewdef() in PG 14-15 returns SELECT dept_emp.emp_no (table-qualified)
  • In PG 16+, the same view returns SELECT emp_no (simplified)
  • This difference caused test failures on PG 14-15

Solution:
Implemented version-specific normalization in ir/normalize.go:

  1. Extract PostgreSQL major version from IR metadata
  2. For PG 14-15 only: normalize view definitions by removing table qualifications from column references
  3. Preserve alias qualifications (e.g., l.emp_no where l is an alias)
  4. Only strip qualifications matching actual table names from FROM/JOIN

Implementation Details:

  • extractPostgreSQLMajorVersion(): Parse version from metadata string
  • normalizeView(): Apply normalization conditionally for PG 14-15
  • normalizeViewDefinitionPG14_15(): Remove table.column qualifications
  • extractTableNamesFromView(): Identify table names vs aliases

Test Results:

  • PostgreSQL 14: PASS ✓
  • PostgreSQL 15: PASS ✓
  • PostgreSQL 16: PASS ✓
  • PostgreSQL 17: PASS ✓

Example Transformation (PG 14-15 only):
Before: SELECT dept_emp.emp_no, max(dept_emp.from_date) ... After: SELECT emp_no, max(from_date) ...

Preserves: SELECT l.emp_no, d.dept_no (aliases remain qualified)

🤖 Generated with Claude Code

PostgreSQL 14 and 15's pg_get_viewdef() returns table-qualified column
names when views are created with schema-qualified table references in
the FROM clause. PostgreSQL 16+ simplified this behavior to return
unqualified column names.

Root Cause:
- When a view is created with `FROM public.dept_emp`, pg_get_viewdef()
  in PG 14-15 returns `SELECT dept_emp.emp_no` (table-qualified)
- In PG 16+, the same view returns `SELECT emp_no` (simplified)
- This difference caused test failures on PG 14-15

Solution:
Implemented version-specific normalization in ir/normalize.go:
1. Extract PostgreSQL major version from IR metadata
2. For PG 14-15 only: normalize view definitions by removing table
   qualifications from column references
3. Preserve alias qualifications (e.g., l.emp_no where l is an alias)
4. Only strip qualifications matching actual table names from FROM/JOIN

Implementation Details:
- extractPostgreSQLMajorVersion(): Parse version from metadata string
- normalizeView(): Apply normalization conditionally for PG 14-15
- normalizeViewDefinitionPG14_15(): Remove table.column qualifications
- extractTableNamesFromView(): Identify table names vs aliases

Test Results:
- PostgreSQL 14: PASS ✓
- PostgreSQL 15: PASS ✓
- PostgreSQL 16: PASS ✓
- PostgreSQL 17: PASS ✓

Example Transformation (PG 14-15 only):
Before: SELECT dept_emp.emp_no, max(dept_emp.from_date) ...
After:  SELECT emp_no, max(from_date) ...

Preserves: SELECT l.emp_no, d.dept_no (aliases remain qualified)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings October 31, 2025 17:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds PostgreSQL version-aware view definition normalization to handle differences in pg_get_viewdef() output between PostgreSQL versions. Specifically, it addresses table-qualified column names in PostgreSQL 14-15 view definitions that don't appear in PostgreSQL 16+.

Key changes:

  • Version extraction logic to parse PostgreSQL major version from metadata
  • Conditional view normalization for PostgreSQL 14 and 15 that removes table qualifications from column references
  • Helper functions to extract table names from view definitions and apply regex-based normalization

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


majorStr := versionPart[:dotIndex]
major := 0
fmt.Sscanf(majorStr, "%d", &major)
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error return value from fmt.Sscanf is ignored. If parsing fails (e.g., non-numeric string), the function returns 0 which is treated as an unknown version. Consider checking the error or using strconv.Atoi for clearer error handling, or add a comment explaining why ignoring the error is acceptable since 0 is already the default for unknown versions.

Copilot uses AI. Check for mistakes.
Comment on lines +318 to +325
for _, existing := range tableNames {
if existing == tableName {
found = true
break
}
}
if !found {
tableNames = append(tableNames, tableName)
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate detection uses a linear search for each table name. For view definitions with many tables or repeated JOIN clauses, this could be inefficient. Consider using a map[string]bool to track seen table names, or use a map and convert to a slice at the end.

Copilot uses AI. Check for mistakes.
@tianzhou tianzhou merged commit abedcc8 into main Oct 31, 2025
8 checks passed
tianzhou added a commit that referenced this pull request Oct 31, 2025
)

PostgreSQL 14 and 15's pg_get_viewdef() returns table-qualified column
names when views are created with schema-qualified table references in
the FROM clause. PostgreSQL 16+ simplified this behavior to return
unqualified column names.

Root Cause:
- When a view is created with `FROM public.dept_emp`, pg_get_viewdef()
  in PG 14-15 returns `SELECT dept_emp.emp_no` (table-qualified)
- In PG 16+, the same view returns `SELECT emp_no` (simplified)
- This difference caused test failures on PG 14-15

Solution:
Implemented version-specific normalization in ir/normalize.go:
1. Extract PostgreSQL major version from IR metadata
2. For PG 14-15 only: normalize view definitions by removing table
   qualifications from column references
3. Preserve alias qualifications (e.g., l.emp_no where l is an alias)
4. Only strip qualifications matching actual table names from FROM/JOIN

Implementation Details:
- extractPostgreSQLMajorVersion(): Parse version from metadata string
- normalizeView(): Apply normalization conditionally for PG 14-15
- normalizeViewDefinitionPG14_15(): Remove table.column qualifications
- extractTableNamesFromView(): Identify table names vs aliases

Test Results:
- PostgreSQL 14: PASS ✓
- PostgreSQL 15: PASS ✓
- PostgreSQL 16: PASS ✓
- PostgreSQL 17: PASS ✓

Example Transformation (PG 14-15 only):
Before: SELECT dept_emp.emp_no, max(dept_emp.from_date) ...
After:  SELECT emp_no, max(from_date) ...

Preserves: SELECT l.emp_no, d.dept_no (aliases remain qualified)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <noreply@anthropic.com>
tianzhou added a commit that referenced this pull request Oct 31, 2025
)

PostgreSQL 14 and 15's pg_get_viewdef() returns table-qualified column
names when views are created with schema-qualified table references in
the FROM clause. PostgreSQL 16+ simplified this behavior to return
unqualified column names.

Root Cause:
- When a view is created with `FROM public.dept_emp`, pg_get_viewdef()
  in PG 14-15 returns `SELECT dept_emp.emp_no` (table-qualified)
- In PG 16+, the same view returns `SELECT emp_no` (simplified)
- This difference caused test failures on PG 14-15

Solution:
Implemented version-specific normalization in ir/normalize.go:
1. Extract PostgreSQL major version from IR metadata
2. For PG 14-15 only: normalize view definitions by removing table
   qualifications from column references
3. Preserve alias qualifications (e.g., l.emp_no where l is an alias)
4. Only strip qualifications matching actual table names from FROM/JOIN

Implementation Details:
- extractPostgreSQLMajorVersion(): Parse version from metadata string
- normalizeView(): Apply normalization conditionally for PG 14-15
- normalizeViewDefinitionPG14_15(): Remove table.column qualifications
- extractTableNamesFromView(): Identify table names vs aliases

Test Results:
- PostgreSQL 14: PASS ✓
- PostgreSQL 15: PASS ✓
- PostgreSQL 16: PASS ✓
- PostgreSQL 17: PASS ✓

Example Transformation (PG 14-15 only):
Before: SELECT dept_emp.emp_no, max(dept_emp.from_date) ...
After:  SELECT emp_no, max(from_date) ...

Preserves: SELECT l.emp_no, d.dept_no (aliases remain qualified)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <noreply@anthropic.com>
tianzhou added a commit that referenced this pull request Oct 31, 2025
tianzhou added a commit that referenced this pull request Oct 31, 2025
* Revert "fix: normalize view definitions for PostgreSQL 14-15 compatibility (#138)"

This reverts commit 46a8759.

* feat: add version-specific test skipping for PostgreSQL 14-15

Add comprehensive test skip mechanism to handle non-consequential view
formatting differences in PostgreSQL 14-15 vs 16+.

Root cause: pg_get_viewdef() in PG 14-15 returns table-qualified column
names (e.g., employees.id) while PG 16+ returns simplified names (e.g., id).
This causes test comparison failures but does not impact correctness.

Changes:
- Add GetMajorVersion() helper in testutil/postgres.go
- Create skip list in testutil/skip_list.go with 13 test patterns
- Add version detection and skip logic to all test files:
  * cmd/migrate_integration_test.go (TestPlanAndApply)
  * cmd/dump/dump_integration_test.go (dump integration tests)
  * cmd/include_integration_test.go (TestIncludeIntegration)
  * internal/diff/diff_test.go (TestDiffFromFiles)

Skipped tests on PG 14-15:
- View tests (create_view/*)
- Materialized view tests (create_materialized_view/*)
- Online materialized view index tests
- Comment tests involving views
- Migration tests v4 and v5
- Dump integration tests (Employee, Issue82ViewLogicExpr)
- Include integration test

All tests continue to run normally on PG 16-17.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* refactor: simplify skip logic to use exact match only

Improvements:
- Remove boolean return value from ShouldSkipTest() - function now only calls t.Skipf()
- Add missing test "create_view/add_view_join" to skip lists
- Simplify matching logic to exact match only (no prefix matching)
- Update all call sites to remove unnecessary if checks

This eliminates potential false positives from prefix matching and makes
the behavior more predictable and maintainable.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* refactor: deduplicate skip list for PG 14-15

Changes:
- Create single skipListPG14_15 variable shared by both versions
- Remove duplicate entries between version 14 and 15
- Both versions now reference the same list via map

This eliminates ~20 lines of duplication and makes maintenance easier.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants